fix: unify duplicate-key error detection across routes#92
Conversation
Extracts the postgres unique-violation check into src/lib/server/services/errors.ts so all four insert paths share the same matcher. The members route previously only matched 'unique' and missed the 'duplicate key' wording, which surfaced as a 500 to portal users adding a duplicate-email member instead of the intended 409. Closes #89
![dobby-coder[bot]](https://avatars.githubusercontent.com/in/3365457?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — would approve if GitHub allowed self-approval on bot-authored PRs.
- Helper at
src/lib/server/services/errors.tsconsolidates the four inconsistent in-line checks; case-insensitive haystack acrossmessage+cause.messagematches the design from #89. - Members route gap is closed: previously only matched
unique, now also catchesduplicate key. - Verified no leftover ad-hoc checks in
src/(only schema.unique()constraints and a migration comment remain). npx vitest run→ 60/60 pass (8 new intests/unit/errors.test.tscovering both wordings, cause-wrapping, case-insensitivity, plain{message}objects, and non-error inputs).npm run check→ 0 errors, 0 warnings.npm run lint→ clean.
Minor (non-blocking): the register route's old check looked at err.stack ?? err.message. For standard Error objects stack always contains the message, so behaviour is equivalent in practice — flagging only in case a custom error class ever overrides stack.
There was a problem hiding this comment.
Re-reviewed (review cycle 1). Verdict from the prior review still stands: PR matches issue #89, all four call sites updated, members-route gap closed, helper handles Error/string/plain-object inputs case-insensitively over message + cause. 8 unit tests pass, CI green across Build/Lint/Svelte Check/Tests/Migration Safety. Cannot --approve (GitHub blocks self-review on bot-authored PRs); treating this as approval-equivalent — ready for a human merge.
There was a problem hiding this comment.
Cycle 2: head 20e4b0c unchanged since cycle 1; re-verified tests (60/60), svelte-check (0/0), lint (clean), CI green. Verdict from prior reviews stands — approval-equivalent, ready for a human to merge. (Self-approve still blocked by GitHub.)
1 participant
Summary
isUniqueViolation(err)insrc/lib/server/services/errors.ts— checks bothmessageandcause.message(case-insensitive) forduplicate keyorunique.unique, so a postgres unique-violation surfacing only asduplicate key ...became a 500 instead of the intended 409. That gap is closed.Test plan
npx vitest run— 60 passed (8 new intests/unit/errors.test.ts)npm run check— 0 errors, 0 warningsnpx vite build— succeeds (adapter-node bundling included)Closes #89